fix: support fork PR intent comments - #205
Merged
Merged
Conversation
z2z23n0
marked this pull request as ready for review
July 27, 2026 10:02
There was a problem hiding this comment.
Pull request overview
This PR updates Mainline’s PR intent comment pipeline to safely support fork-based pull requests by shifting the GitHub Actions workflow to pull_request_target and adding an engine/CLI pathway that reads fork-published intent metadata as Git data without importing it into upstream state.
Changes:
- Added
PRCommentWithOptionsand CLI flags (--pr,--fork-url) to render fork actor-log intent metadata in PR comments without syncing/importing. - Introduced fork intent discovery + rendering (including provenance messaging) and added targeted engine/CLI tests for fork behavior.
- Migrated the
Mainline PR Intentworkflow to a singlepull_request_targetflow with concurrency cancellation and stale-head checks.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/engine/pr.go | Adds PullRequestCommentOptions + fork-aware PR comment rendering entrypoint. |
| internal/engine/pr_fork.go | Implements fork actor-log discovery/fetch and merges fork intents into PR comment view. |
| internal/engine/pr_fork_test.go | Adds regression tests ensuring fork intent preview doesn’t import/accept upstream state and filters terminal intents. |
| internal/cli/pr.go | Wires new pr-comment options (--pr, --fork-url) to the engine options struct. |
| internal/cli/root_test.go | Ensures new pr-comment flags are registered. |
| .github/workflows/mainline-pr-intent.yml | Switches to pull_request_target, adds concurrency, stale event checks, and fork-safe checkout/fetch strategy. |
Comments suppressed due to low confidence (3)
internal/engine/pr_fork.go:121
- Similarly, decoding failures for specific fork event payloads (sealed/abandoned/superseded) currently abort the entire PR comment generation. Since these payloads are also untrusted, treat decode failures as a skip for that event instead of failing the run.
case domain.EventIntentSealed:
var event domain.IntentSealedEvent
if err := json.Unmarshal(raw, &event); err != nil {
return nil, domain.NewError(
domain.ErrInvalidInput,
fmt.Sprintf("fork sealed event %s is invalid: %v", base.EventID, err),
)
}
internal/engine/pr_fork.go:133
- For fork PR previews, an invalid abandoned event payload should not fail the entire PR intent comment generation (untrusted fork input). Skip malformed abandoned events instead.
case domain.EventIntentAbandoned:
var event domain.IntentAbandonedEvent
if err := json.Unmarshal(raw, &event); err != nil {
return nil, domain.NewError(
domain.ErrInvalidInput,
fmt.Sprintf("fork abandoned event %s is invalid: %v", base.EventID, err),
)
}
internal/engine/pr_fork.go:145
- For fork PR previews, an invalid superseded event payload should not fail the entire PR intent comment generation (untrusted fork input). Skip malformed superseded events instead.
case domain.EventIntentSuperseded:
var event domain.IntentSupersededEvent
if err := json.Unmarshal(raw, &event); err != nil {
return nil, domain.NewError(
domain.ErrInvalidInput,
fmt.Sprintf("fork superseded event %s is invalid: %v", base.EventID, err),
)
}
Comment on lines
+214
to
+219
| func forkPRCommentImportRef(actorID string, prNumber int) string { | ||
| if prNumber > 0 { | ||
| return fmt.Sprintf("refs/mainline/imports/pr-comments/pr-%d/%s/log", prNumber, actorID) | ||
| } | ||
| return "refs/mainline/imports/pr-comments/manual/" + actorID + "/log" | ||
| } |
Comment on lines
+100
to
+108
| intents := make(map[string]domain.IntentView) | ||
| for index, raw := range rawEvents { | ||
| var base domain.BaseEvent | ||
| if err := json.Unmarshal(raw, &base); err != nil { | ||
| return nil, domain.NewError( | ||
| domain.ErrInvalidInput, | ||
| fmt.Sprintf("fork actor log event %d is not valid JSON: %v", index, err), | ||
| ) | ||
| } |
Comment on lines
+82
to
+86
| with: | ||
| ref: ${{ github.event.pull_request.base.sha }} | ||
| fetch-depth: 0 | ||
| persist-credentials: false | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mainline Intent
Intent:
int_807c5590Status:
proposedTitle: 为 Mainline PR Intent Action 增加 fork PR 支持
What changed
将 Mainline PR Intent workflow 迁移为单一 pull_request_target 路径:同仓 PR 继续 checkout 并执行 head 代码,fork PR 只 checkout 可信 base,并把 PR head 作为 Git 数据读取。pr-comment 新增可选的 fork URL/PR 参数,可从 fork actor refs 临时投影 contributor-published intents,复用原有 commit-range、多 intent、branch fallback、marker 和 sticky comment 行为。加入并发取消、head 新鲜度校验、live PR body 检查和临时 ref 清理,并补充 engine/CLI 回归测试。
Why
普通 pull_request 在 fork PR 上只有只读 token,无法写评论;直接移除同仓限制又会让高权限 job 执行不可信 fork 代码。新的可信 base 执行路径在不改变现有同仓评论能力的前提下安全读取 fork intent metadata,并避免在 review 阶段导入或修改 upstream Mainline 状态。
Decisions
Subsystems: .github, cli, engine, github-actions, pull-request, intent-rendering, fork-actor-log